-
Notifications
You must be signed in to change notification settings - Fork 118
[WCiOS17] Update PaymentMethodsView to not use deprecated NavigationLink init
#16196
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Seems to crash the previews macro if we do not provide a rootViewController, despite being optional
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works well, thanks for fixing these deprecated usage! 🚀
| .previewDisplayName("Light") | ||
| #Preview("Light") { | ||
| @Previewable @State var rootViewController = UIViewController() | ||
| NavigationView { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: as NavigationView is deprecated, would be nice to replace it with NavigationStack. Simply replacing it with NavigationStack seems to work. Similarly for other previews in this file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's very true, updated: 84d2e6a
| ) | ||
| .navigationBarTitleDisplayMode(.inline) | ||
| } | ||
| .environment(\.colorScheme, .dark) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: unrelated to this PR, I think we could just keep one preview here since the differences are just the color scheme and accessibility font size which can be configured in SwiftUI previews. It's more worth having multiple previews when the data/states are different.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that sounds good to me. Updated: 84d2e6a
|
Version |

Closes WOOMOB-1406
Description
As with #16195, this PR updates the
PaymentMethodsViewcomponent from Order > Collect Payment to not use deprecatedNavigationLink(isActive:)API for navigation. It also fixes the file previews, which seem to crash in Xcode due missing dependencies.Changes
NavigationLink(isActive:in favor of.navigationDestination(isPresented:view modifierTesting information
Cashas payment methodScreenshots